Skip to content

Add discussion of runtime errors and refusal-to-process vs validation success/failure #1316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

handrews
Copy link
Contributor

Fixes #1276 regarding the lack of discussion of runtime errors.

This intentionally avoids saying anything at all about the mechanism or format of any of the outcomes.

Comment on lines +623 to +627
runtime error: the schema was understood but the evaluation could not
complete
</t>
<t>
refusal to process: the schema was not understood and evaluation never began
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that these need to be distinct. I think the outcome of a "refusal to process" is still a runtime error.

In .Net, a "runtime error" means that an exception would be thrown. This is the outcome in both cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're distinct because the expected responses are distinct (and this might actually mean I have these definitions a bit wrong in the current text).

A "refusal to process" is a correct outcome. There is nothing you can or should do to produce a different outcome. If you do something like remove an unsupported vocabulary that was preventing evaluation, you're really just trying something else entirely. It might be represented by a validation status of null or None or whatever, instead of true or false. It could be an exception, but it's not really something that is handled.

A "runtime error" is an incorrect outcome. It indicates either an error in the schema (referencing a non-existent internal location, which can be determined immediately) or an error in configuration (failing to supply the necessary external schema for resolving references). These are correctable errors- exceptions you can potentially handle.

For example, if you get an exception that says that a reference to https://example.com/external-schema#/$defs/whatever cannot be resolved, you might be able to download that resource, load it, and re-try the original evaluation.

Does this correct-but-no vs correctable-error distinction help? I am definitely open to re-framing this, "runtime error" and "refusal to process" were just what came to mind, and the definitions were off the top of my head.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... That's subtle, and I don't think I'd go through the effort of making the distinction except maybe throwing different exception types. But I'd still throw an exception for both cases.

If the spec is going to identify "refusal to process" as a non-erroring result, then accommodation needs to be made for it in the output. Currently there is no such accommodation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also identify the specific scenarios where "refusal to process" is the desired result (as we have with $vocabulary).

"Runtime error" should be the catch-all result for anything not specified as one of the other result states.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gregsdennis

We should also identify the specific scenarios where "refusal to process" is the desired result (as we have with $vocabulary).

I like this idea. I think I got too caught up in when things can happen (schema load or evaluation) which produced a weird ambiguity/overlap in these categories.

An unresolvable reference should always be a "runtime error" even if it is detected at schema load time.

I don't think implementation details such as what is or is not an exception (some languages don't even have exceptions, some implementations don't use them) are relevant, though. I wouldn't put refusal-to-process in the output format, but I'm OK with deferring that decision until later (I wouldn't fight it tooth and nail either- it's a weak preference).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with that. Sounds like the paragraph at line 642 needs to be updated, then, as it attempts to make the load/evaluation distinction.

Honestly, I'm not sure we need it at all unless you want to CREF it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gregsdennis I removed it and rewrote the previous paragraph- please see if this seems clear and reasonable.

Copy link
Member

@karenetheridge karenetheridge Oct 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the distinction between "runtime error" and "refuse to process" is hazy, especially since I think that encountering an unknown $vocabulary fits into both categories. You say that "refuse to process" means that nothing you can do would provide a better outcome, whereas an unresolvable $ref is fixable (e.g. by loading the missing resource). But isn't that the same as a missing $vocabulary? I could resolve that error by informing the evaluator about that vocabulary (that is, perform some extra bit of code to load that vocabulary logic into the evaluator).

Perhaps you're seeing the distinction as between "we can detect this condition by statically analyzing the schema", or "we don't know if the evaluation can be run to completion until we try to evaluate it against an instance"? We can certainly detect a missing $schema or $vocabulary in static analysis; whether or not you choose to detect a missing $ref at runtime is more tricky (certainly if you allow for loading resources from disk/network at runtime, you could delay this determination; also, even if all schemas need to be preloaded, circular references mean that one cannot immediately error on loading a schema if it contains an unknown $ref, as that resource may be added sometime in the future but still in time for a successful evaluation).

The point at which the condition is detected should not change
the type of outcome.
detection of an infinite reference loop, or excessive consumption of memory.
</t>
<t>
Unlike runtime errors, refusal-to-process is the expected outcome in certain cases.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stil think this line is a bit fuzzy. Consider parsing a number from a string. Failing to parse "123A5" could be considered a refusal to process because

  • the input was invalid, or
  • the request to parse should have specified hexadecimal

In both cases, the outcome is failure external to the normal processing of a function.

Similarly, refusal to process and runtime failures both have an outcome of a failure that is external to the normal processing of a schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gregsdennis what do you suggest? There is clearly a difference between "I asked for a feature to be supported and it's not so I'm going to do the sensible thing and not use this schema" and "oops something that was expected to work didn't work." What way of describing these things would you find appropriate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's my point: the difference is semantic. It's the difference between two errors, and I don't think that the specification needs to define that difference. The specification just needs to say, "validation cannot proceed in this case," and let the implementation decide what that means for its language.

A feature not being supported is not expected. (In fact, .Net has NotSupportedException for that specific case.) I can't think of reason why you'd want an application where evaluation is required to continue processing if that evaluation errors, whether it's for bad JSON, an unresolvable $ref, or an unknown vocab. It's all the same outcome: something is wrong and the application has to stop (or gracefully catch the error and inform the user).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two phases here: "Can I use this?" and "I'm using this." The way to react to failures in each is different.

What is the advantage in preventing these from being distinct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other part of this is that the spec intentionally requires a "refuse to process" behavior.

What's up for debate is not whether that is a behavior, as that was part of a past debate and was written into the spec. What is up for debate now is how to explain "refuse to process" compared to other conditions.

PRs are not the place to re-litigate past decisions. This PR just needs to clarify what's there. Changing what's there requires a new issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are different outcomes. One is directed at the person who tried to use the schema and tells them to go load a plugin or something. It is specifically called out in the spec as a distinct outcome. The other types of errors tell the person who wrote the schema that something is wrong with the schema.

  • Different treatment in the specification language
  • Different audiences for the errors
  • Different types of corrective action
  • Different characterization of the schema (incorrectly written vs correctly written)

These are different things. In every dimension I can think of. BTW, I do not care what form the errors take. You can just throw different exceptions, that's fine with me- nothing in this PR has anything to do with how they are reported. But everything about these categories of outcomes is different.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One is directed at the person who tried to use the schema and tells them to go load a plugin or something. It is specifically called out in the spec as a distinct outcome. The other types of errors tell the person who wrote the schema that something is wrong with the schema.

When evaluating a schema, the author of the schema is (theoretically) uninvolved and likely will not receive notification that something went wrong. The implementation's only recourse is to halt and report to the user why it halted.

Maybe it halts differently, I guess, but it still halts. Whether the implementation chooses to not process a schema or it can't process a schema is irrelevant. The outcome is that the schema isn't processed and must report some sort of failure. The messaging in the failure tells the user

  • the nature of the problem ("Different characterization of the schema (incorrectly written vs correctly written)")
  • if they can do anything about it ("Different audiences for the errors")
  • what to do ("Different types of corrective action")

It's just like a JSON parser that can handle comments but isn't configured to do so. When it encounters comments, it stops and reports what's wrong.

My point is that processing halts. I don't think that the specification needs to say anything more than that.

Are you saying that the specification should prescribe the (minimum) content of a failure (if not the mechanism, e.g. an exception)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or.... are you saying that you want to distinguish between errors that must be caught at evaluation-time and errors which could be caught in some sort of static analysis? Would that require implementations to support static analysis (mine doesn't)?

Copy link
Member

@karenetheridge karenetheridge Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's how I view it -- there are two main kinds of exceptions:

  • compile time exceptions: something is written incorrectly in the schema (mostly things we would catch by evaluating against the metaschema, although there are a few things in the spec that can't quite be expressed with the metaschema, as we've discovered.. no I'm not looking up a reference to insert here). These things can all be determined by statically inspecting the schema in isolation from any data instance, and, I think it could be argued, also in isolation independently from any particular implementation
  • runtime exceptions: something went wrong while actually doing something with the schema (e.g. evaluating it against a data instance, using it to generate code or documentation), etc. some of these things might be localized to a particular implementation at a point in time (e.g. you forgot to load a supporting library that would let you validate domain names), or a particular state of the local resources (you have no internet connection, so you can't resolve a $ref that points to a non-local resource).

It might also be worthwhile to distinguish between transient and permanent errors. In my implementation, I try to only error at "compile time" for things that are not correctable (e.g. syntax errors), and anything that could be corrected between schema-loading time and data-instance-evaluation time is deferred until runtime (loading optional modules for format handling, checking if the URI in a $ref actually resolves to a known schema location, etc).

Would that require implementations to support static analysis (mine doesn't)?

When you load a schema into your implementation, do you generate an error if the schema is malformed (e.g. "allOf":"not an array"? Or do you do nothing with it until it's time to evaluate against a data instance?
You can also inspect a schema for errors simply by evaluating it against its metaschema.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you load a schema into your implementation, do you generate an error if the schema is malformed (e.g. "allOf":"not an array"?

Yes, but they would be considered a deserialization (JSON text -> model) error. I don't see that as static analysis.

I think for this discussion we can assume that we have a syntactically valid schema.

Evaluation against a meta-schema does (optionally) happen, but a lot of that is baked into the deserialization because the models are strongly typed and literally can't accept invalid values.

@handrews
Copy link
Contributor Author

Given that:

  • no one has even attempted to address the most important point that "refusal to process" has been in the spec as a distinct outcome since 2019, and this is simply a clarification of what is there
  • no one seems interested in what I consider the 2nd most important point here which is the different audiences for the different outcomes
  • the subsequent discussion has gone in a completely different direction than anything I was proposing

I am abandoning this PR.

@handrews handrews closed this Nov 20, 2022
@gregsdennis gregsdennis added clarification Items that need to be clarified in the specification and removed Type: Maintenance labels Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Items that need to be clarified in the specification core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Behavior around failed $ref resolution is not defined
3 participants